Skip to content

perf: eliminate redundant getHash calls in HashMap/HashSet concat#26278

Merged
SolalPirelli merged 1 commit into
scala:mainfrom
He-Pin:perf/hashmap-hashset-concat-gethash
Jun 11, 2026
Merged

perf: eliminate redundant getHash calls in HashMap/HashSet concat#26278
SolalPirelli merged 1 commit into
scala:mainfrom
He-Pin:perf/hashmap-hashset-concat-gethash

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Description

Eliminate redundant getHash calls in HashMap/HashSet concat.

Problem

In BitmapIndexedMapNode.concat and BitmapIndexedSetNode.concat, the leftNodeRightData branch calls bm.getHash(rightDataIdx) twice: once to store in rightOriginalHash, and again as the originalHash argument to updated(). The second call is redundant since the value is already available.

Solution

Replace the second bm.getHash(rightDataIdx) with rightOriginalHash in both HashMap and HashSet concat implementations.

Result

Eliminates one redundant array access per right-side data element during concat operations in the leftNodeRightData branch.

LLM Policy

LLM-based tools were used extensively in this contribution. The code was reviewed and tested locally before submission.

Motivation:
In BitmapIndexedMapNode.concat and BitmapIndexedSetNode.concat, the
leftNodeRightData branch calls bm.getHash(rightDataIdx) twice: once
to store in rightOriginalHash, and again as the originalHash argument
to updated(). The second call is redundant since the value is already
available.

Modification:
Replace the second bm.getHash(rightDataIdx) with rightOriginalHash
in both HashMap and HashSet concat implementations.

Result:
Eliminates one redundant array access per right-side data element
during concat operations in the leftNodeRightData branch.
@He-Pin He-Pin marked this pull request as ready for review June 8, 2026 17:19
@He-Pin He-Pin requested a review from a team as a code owner June 8, 2026 17:19

@SolalPirelli SolalPirelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'm good with this as a cleanup regardless of perf

@SolalPirelli SolalPirelli merged commit 8b7e2ce into scala:main Jun 11, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants